feat: add a remove button element for semantic#900
feat: add a remove button element for semantic#900zombieJ merged 8 commits intoreact-component:masterfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough将 Tab 组件的样式/类名 API 从单一的 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as 应用
participant Tabs as Tabs
participant Nav as TabNavList
participant Node as TabNode
participant Op as OperationNode
App->>Tabs: 提供 props (classNames, styles, editable, ...)
Tabs->>Nav: 转发 tabs 列表与语义化 classNames/styles
Note over Tabs,Nav: 语义键包含 item / remove / ...
loop 每个 tab
Nav->>Node: render(tab, { classNames: {item, remove}, styles: {item, remove}, ... })
Node->>Node: 使用 classNames?.item / styles?.item 构建项 DOM
alt 可编辑且显示移除
Node->>Node: 使用 classNames?.remove / styles?.remove 渲染移除按钮
end
end
Nav->>Op: 溢出菜单渲染时传入 classNames?.remove / styles?.remove
Op->>Op: 下拉项应用 remove 语义的类与样式
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @coding-ice, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the customization capabilities for tab components by introducing a dedicated semantic name for the close button. It refactors the styling and class name props within the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds the ability to style the close button on tabs by introducing a 'close' semantic name. The implementation is well-done and consistent. My main feedback is the lack of updated tests. The new functionality for classNames.close and styles.close should be covered by unit tests to prevent future regressions. I've also left one minor suggestion to improve type consistency.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/TabNavList/index.tsx (1)
438-447: 按语义传递 item/close 的类名与样式 — LGTM；可微调合并以更健壮在严格类型设置下（如 exactOptionalPropertyTypes），展开
styles?.item可能触发类型告警。可使用空对象兜底，提升健壮性（行为不变）。- item: i === 0 ? styles?.item : { ...tabNodeStyle, ...styles?.item }, + item: i === 0 ? styles?.item : { ...tabNodeStyle, ...(styles?.item || {}) },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/TabNavList/TabNode.tsx(5 hunks)src/TabNavList/index.tsx(1 hunks)src/Tabs.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/TabNavList/TabNode.tsx (1)
src/Tabs.tsx (1)
SemanticName(37-37)
🔇 Additional comments (7)
src/Tabs.tsx (1)
37-37: 扩展 SemanticName 包含 'close' — LGTM与下游 TabNode/TabNavList 改动一致。
src/TabNavList/TabNode.tsx (3)
87-94: 为 item 节点应用语义化类名与样式 — LGTM
tabNodeClassNames?.item与styles?.item的解构与落点正确。
134-136: 为 close 按钮应用语义化类名与样式 — LGTM
tabNodeClassNames?.close与styles?.close使用恰当。
5-5: 改为相对导入或确认 '@' 路径别名已配置仓库搜索仅在 src/TabNavList/TabNode.tsx:5 发现 '@/Tabs' 引用，未找到 tsconfig 的 paths 或其它别名声明；建议改为相对导入以保持一致性：
-import type { SemanticName } from '@/Tabs'; +import type { SemanticName } from '../Tabs';若需保留 '@' 别名，确认在 tsconfig.json 或构建配置（vite/webpack）中添加 "@/": ["src/"] 并确保构建/类型检查能解析该别名。
src/TabNavList/index.tsx (3)
593-595: header 语义的 className/style 透传 — LGTM顺序为用户
style覆盖语义header，优先级合理。
633-637: indicator 语义的 className/style 透传 — LGTM
indicatorStyle与自定义样式合并顺序合理。
650-651: 确认：OperationNode 已声明并使用 popupStyle，可直接传入src/TabNavList/OperationNode.tsx 声明了 popupStyle?: React.CSSProperties（约第29行），并在下拉中以 overlayStyle={popupStyle} 使用（约第198行），因此在 src/TabNavList/index.tsx 传入 popupStyle={styles?.popup} 是安全的。
coding-ice
force-pushed
the
feat/semantic
branch
from
8c4f5fd to
c9d46f4
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #900 +/- ##
==========================================
- Coverage 98.98% 98.98% -0.01%
==========================================
Files 18 18
Lines 789 788 -1
Branches 237 228 -9
==========================================
- Hits 781 780 -1
Misses 8 8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
coding-ice
force-pushed
the
feat/semantic
branch
from
0285c3e to
f0f7e8c
Compare
|
close 按钮有两个，好像漏了一个地方 |
coding-ice
changed the title
弹出层的确实漏下了，我补一下～ |
我不确定这两个地方的 close 是不是要用同一个语义，可以问问 @zombieJ |
|
@coding-ice is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
Co-authored-by: lijianan <[email protected]>
…sNames and styles for close button
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/index.test.tsx (1)
779-807: 简化测试设置以提高清晰度。测试逻辑正确，但设置可能过于复杂。使用窄容器（100px）和 10 个标签项通常会触发溢出行为，但此测试仅检查可见标签的关闭按钮（
.rc-tabs-tab-remove），而不是溢出下拉菜单中的按钮。建议简化测试设置，使用较少的标签项和正常容器宽度，因为此测试的重点是可见标签的关闭按钮样式，而不是溢出行为。
可以考虑应用以下简化：
it('support classnames and styles for editable close button', () => { const customClassNames = { close: 'custom-close', }; const customStyles = { close: { background: 'red' }, }; const { container } = render( - <div style={{ width: 100 }}> - <Tabs - editable={{ - onEdit: () => {}, - }} - tabPosition="left" - items={Array.from({ length: 10 }).map((_, index) => ({ - key: `test-${index}`, - label: `test-${index}`, - icon: 'test', - }))} - styles={customStyles} - classNames={customClassNames} - /> - </div>, + <Tabs + editable={{ + onEdit: () => {}, + }} + items={[ + { key: 'test-1', label: 'test-1', icon: 'test' }, + { key: 'test-2', label: 'test-2', icon: 'test' }, + ]} + styles={customStyles} + classNames={customClassNames} + />, ); expect(container.querySelector('.rc-tabs-tab-remove')).toHaveClass('custom-close'); expect(container.querySelector('.rc-tabs-tab-remove')).toHaveStyle({ background: 'red' }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/index.test.tsx(1 hunks)tests/overflow.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/overflow.test.tsx (1)
tests/common/util.tsx (2)
getTabs(113-147)triggerResize(149-156)
🔇 Additional comments (1)
tests/overflow.test.tsx (1)
531-556: 测试实现正确，补充了溢出下拉菜单的关闭按钮样式测试。此测试正确地验证了溢出下拉菜单中关闭按钮的自定义样式和类名应用。测试流程清晰：
- 设置可编辑标签和自定义样式
- 触发溢出行为
- 打开下拉菜单
- 验证下拉菜单中的关闭按钮应用了自定义样式
这正是 PR 评论中提到的"弹出层"部分，与
tests/index.test.tsx中的测试互补，共同覆盖了两种场景下的关闭按钮样式。基于 PR 评论。
src/TabNavList/OperationNode.tsx
Outdated
| aria-label={removeAriaLabel || 'remove'} | ||
| tabIndex={0} | ||
| className={`${dropdownPrefix}-menu-item-remove`} | ||
| className={clsx(`${dropdownPrefix}-menu-item-remove`, classNames?.close)} |
There was a problem hiding this comment.
这里的 close 语义，不太确定是不是要改为 popup.close
…larity and consistency
coding-ice
changed the title
3 participants
Summary by CodeRabbit
新功能
破坏性变更
测试